Fixes nested dependency detection in NpmLockfile3Detector and applies the engines array fix that was missing from PR #1400. #1606

gzproger · 2026-01-08T21:00:44Z

Changes

Rewrote ProcessLockfile to iterate ALL packages directly (matching npm's actual behavior)
Applied PackageJsonEnginesConverter to PackageLockV3Package.Engines (PR don't throw if engines property is a string array #1400 only fixed NpmComponentDetector, not NpmLockfile3Detector)
Fixed devOptional classification for packages that are both dev and optional dependencies of non-dev packages
Added node-style module resolution for building dependency graph edges

Testing

All unit tests pass (11/11)
Tested on 3 real-world repos with correct component counts

…cation - Rewrite ProcessLockfile to iterate ALL packages directly instead of tree traversal - Add node-style module resolution for dependency graph edges - Fix devOptional and peer+dev dependencies to be classified as dev dependencies - Skip link packages (symbolic links) and bundled dependencies - Handle engines field as JsonElement to support both object and array formats - Add tests for devOptional, peer-only, and engines-as-array scenarios Fixes issue microsoft#1380 ## Test Results Tested against multiple real-world repositories with npm lockfile v3: - Main branch crashes when engines field is array format instead of object - This PR correctly handles both object and array formats for engines - This PR detects all packages from lockfile including nested dependencies - NpmLockfile3 now finds the complete transitive dependency tree

codecov · 2026-01-08T21:04:21Z

Codecov Report

❌ Patch coverage is 95.59939% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.2%. Comparing base (2ed2e3b) to head (36b9efe).

Files with missing lines	Patch %	Lines
...entDetection.Detectors/npm/NpmLockfile3Detector.cs	80.0%	12 Missing and 9 partials ⚠️
...onentDetection.Detectors.Tests/NpmTestUtilities.cs	90.2%	0 Missing and 8 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1606     +/-   ##
=======================================
+ Coverage   90.1%   90.2%   +0.1%     
=======================================
  Files        437     437             
  Lines      37431   38027    +596     
  Branches    2316    2342     +26     
=======================================
+ Hits       33736   34319    +583     
+ Misses      3221    3219      -2     
- Partials     474     489     +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR fixes nested dependency detection in NpmLockfile3Detector and applies the missing engines array converter fix from PR #1400. The core change rewrites the ProcessLockfile method from a queue-based traversal to a cleaner three-pass approach that processes all packages directly, then registers components, and finally builds dependency edges using node-style module resolution.

Key changes:

Replaces complex queue-based dependency traversal with direct iteration over all packages in lockfile
Implements node-style module resolution for dependency graph construction, walking up the directory tree to find dependencies
Fixes devOptional classification to correctly mark peer dependencies that are also dev dependencies

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs	Complete rewrite of ProcessLockfile to use three-pass approach with node-style resolution; removes queue-based traversal
src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs	Adds new RecordComponent overload for registering components with explicit reference flag
src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageLockV3Package.cs	Applies PackageJsonEnginesConverter to Engines property to handle array format
test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs	Adds three new tests for devOptional, peer, and engines array handling
test/Microsoft.ComponentDetection.Detectors.Tests/NpmTestUtilities.cs	Adds helper methods for generating test lockfiles with devOptional and peer dependencies

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs

…ng and handling of link/bundled packages

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs

Copilot · 2026-01-12T18:49:48Z

test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs

+        // The shared dependency appears in both dev and non-dev contexts, so should NOT be marked as dev
+        var sharedDep = detectedComponents.First(x => ((NpmComponent)x.Component).Name.Equals(sharedDepName));
+        componentRecorder.GetEffectiveDevDependencyValue(sharedDep.Component.Id).Should().BeFalse();


The test comment states "The shared dependency appears in both dev and non-dev contexts," but the lockfile structure shows the shared dependency appears only once at "node_modules/{4}" without a dev flag. The multi-path dev status tracking logic (lines 135-138 in NpmLockfile3Detector.cs) is not actually exercised by this test since the component appears at only one path in the lockfile. Consider adding a test case where the same component (name + version) appears at multiple different paths in the lockfile with different dev flags to properly test the multi-path logic.

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs

gzproger · 2026-01-12T18:53:43Z

@copilot open a new pull request to apply changes based on the comments in this thread

…ector.cs Co-authored-by: Copilot <[email protected]>

…ctorBase.cs Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Copilot · 2026-01-12T19:00:11Z

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs


-        // Build package lookup - keys are paths like "node_modules/lodash" or "node_modules/a/node_modules/b"
+        // Collect direct dependencies from package.json for explicit reference tracking
+        var directDependencies = new HashSet<string>(System.StringComparer.Ordinal);


NPM package names are case-sensitive according to the npm specification. Using StringComparer.OrdinalIgnoreCase could lead to incorrect matching of package names. For example, "React" and "react" would be treated as the same package when they are actually different. This should be changed to StringComparer.Ordinal to ensure case-sensitive matching.

gzproger requested a review from a team as a code owner January 8, 2026 21:00

gzproger requested review from AMaini503 and Copilot January 8, 2026 21:00

Copilot started reviewing on behalf of gzproger January 8, 2026 21:01 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs Show resolved Hide resolved

jiewenz and others added 2 commits January 8, 2026 17:27

Refactor NpmLockfile3Detector and tests to improve dependency recordi…

4b9e0d0

…ng and handling of link/bundled packages

Merge branch 'main' into npmlock3-fix-nested-deps

c8282b5

Copilot AI review requested due to automatic review settings January 12, 2026 18:44

Copilot started reviewing on behalf of gzproger January 12, 2026 18:45 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

gzproger closed this Jan 12, 2026

gzproger reopened this Jan 12, 2026

Update src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Det…

e6ec89e

…ector.cs Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings January 12, 2026 18:55

Update src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Det…

9f8a877

…ector.cs Co-authored-by: Copilot <[email protected]>

Copilot started reviewing on behalf of gzproger January 12, 2026 18:55 View session

gzproger and others added 2 commits January 12, 2026 10:56

Update src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Det…

6a8cc4e

…ector.cs Co-authored-by: Copilot <[email protected]>

Update src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDete…

36b9efe

…ctorBase.cs Co-authored-by: Copilot <[email protected]>

Copilot AI reviewed Jan 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixes nested dependency detection in NpmLockfile3Detector and applies the engines array fix that was missing from PR #1400. #1606

Fixes nested dependency detection in NpmLockfile3Detector and applies the engines array fix that was missing from PR #1400. #1606

Uh oh!

gzproger commented Jan 8, 2026

Uh oh!

codecov bot commented Jan 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Uh oh!

Uh oh!

Uh oh!

gzproger commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fixes nested dependency detection in NpmLockfile3Detector and applies the engines array fix that was missing from PR #1400. #1606

Are you sure you want to change the base?

Fixes nested dependency detection in NpmLockfile3Detector and applies the engines array fix that was missing from PR #1400. #1606

Uh oh!

Conversation

gzproger commented Jan 8, 2026

Changes

Testing

Uh oh!

codecov bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gzproger commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jan 8, 2026 •

edited

Loading